-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drpcmetadata: encode and decode metadata #2
Conversation
in order to send tracing information across the wire, we need to extend drpc invoke message to contain key/value pairs. We will set the first two bytes of the message to be empty so we know the message is using the new format and may contain meatadata. Then we will proto buff to encode our invoke message so we can send it across the wire. Change-Id: I03a5af62c5b41e0855dd101faacd8d12c66c2cdb
Change-Id: I7c3d526fdd091c0db7233ec54e0c36f8da634745
Change-Id: I3410aad28ccef787183b74cc537aa71b7d9b973b
Change-Id: Idfeaa81111d63775b8cd8fc41baa4628ff8c6d23
Change-Id: I3e6265735ae6f5859b23cbe1d585931ecff8a69b
Change-Id: Ia9551ba0db65fb4d27ea09ec9847696b6738fb8d
18b5873
to
67a925f
Compare
I still need to work on docs and tests for the new package but I thought it would nice to get the interface reviewed before I do that work. |
67a925f
to
1b4d5ec
Compare
Change-Id: I256df930c969c4c81850d66d85a72488abd5529b
1b4d5ec
to
11139e9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like the drpcmetadata/proto
package name yet, but I don't have a suggestion, either. Maybe it can be named drpcmetadata/invoke
?
drpcconn/conn.go
Outdated
data, err := proto.Marshal(in) | ||
if err != nil { | ||
return errs.Wrap(err) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we move this back up to where it was? I prefer to handle this error before we interact with the manager. The idea is to get all of the locally erroring things done before potentially doing network erroring things.
Change-Id: I8b5854318fa0bb4f0320c330528f5ba5a05c19c5
Change-Id: If6a25354a8956ede9c938f4706e6f7296ca89da6
Change-Id: I49a449e8b5127e30e54d50b30e44fc5fc0293007
Change-Id: I5f6b3d6857e9eb71aa503b2ba85b2f506fdf0a81
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just some nitpicks left.
Change-Id: I91feb4861693eaa70687378f04576de0876df46a
Change-Id: I783fc8ff13d160eed58708d8d670d6718af49b95
Provide interface for adding/retrieving metadata from context across the wire.